Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix JuliaFormatter to 1.0.34 #676

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Fix JuliaFormatter to 1.0.34 #676

merged 1 commit into from
Jun 12, 2024

Conversation

mortenpi
Copy link
Contributor

This downgrades and fixes JuliaFormatter to 1.0.34. I believe this should get rid of the formatting changes in #637. 1.0.34 was the latest JuliaFormatter version when the last API update was merged in #636. My suggestion is to get #637 in with the old formatting, and then figure out how to re-format the generated code.

Note: 1.0.34 doesn't seem to cause any formatting changes on master, so in that sense should be safe to merge.

This was referenced Jun 12, 2024
Merged
Merged
@ararslan
Copy link
Member

1.0.34 doesn't seem to cause any formatting changes on master

1.0.56 (what I get when I just ]add JuliaFormatter) also doesn't seem to produce any changes though 😕

@mortenpi
Copy link
Contributor Author

1.0.56 (what I get when I just ]add JuliaFormatter) also doesn't seem to produce any changes though 😕

On master it doesn't, because they have already been formatted into to function foo(); ...; end form, and it doesn't format them back. But if you re-generate the APIs (which generates the foo() = ... form), then you should see a difference -- 1.0.56 sticks with foo() = ..., but 1.0.34 formats to function foo(); ...; end.

@ararslan
Copy link
Member

Oh I see, thanks. What you have here seems like a reasonable solution then.

@ararslan ararslan merged commit addd076 into JuliaCloud:master Jun 12, 2024
@mortenpi mortenpi deleted the mp/downgrade-formatter branch June 12, 2024 03:43
@mortenpi
Copy link
Contributor Author

I guess #637 should also update shortly, but FWIW, running locally, this is what the diff should look like with the old formatter: master...mortenpi:AWS.jl:mp/api-diff -- the non-substantial changes seem to be gone.

@ararslan
Copy link
Member

That's great! Thanks for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ararslan ararslan ararslan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mortenpi @ararslan